Skip to content

Disallow ? as a type name #8735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2020

Conversation

smarter
Copy link
Member

@smarter smarter commented Apr 16, 2020

Now that we use ? for wildcards, we should treat it more like a
keyword for types and not allow it as a type name to avoid
confusion.

@smarter smarter requested a review from odersky April 16, 2020 18:36
Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the duplication in Parser here. It feels brittle. If we ever think of a new way to create types, I am sure this will slip through and they won't be checked for "?". We could do it in Typer, then it would be just one place, when we typecheck a TypeDef. Did you consider that?

@smarter
Copy link
Member Author

smarter commented Apr 20, 2020

Yeah the duplication isn't great, I didn't think of doing it in typer, I'll give it a go.

@smarter smarter assigned smarter and unassigned odersky Apr 20, 2020
@smarter smarter force-pushed the disallow-tparam-questionmark branch from 7322ab1 to d08b906 Compare April 20, 2020 12:26
@smarter smarter assigned odersky and unassigned smarter Apr 20, 2020
@smarter smarter requested a review from odersky April 20, 2020 13:07
Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

@@ -2321,6 +2321,13 @@ class Typer extends Namer
val typer1 = localTyper(sym)
typer1.typedDefDef(tree, sym)(using ctx.localContext(tree, sym).setTyper(typer1))
case tree: untpd.TypeDef =>
if tree.name eq tpnme.? then
val addendum = if sym.owner.is(TypeParam)
Copy link
Contributor

@odersky odersky Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd move the whole case into a new method typedTypeOrCassDef, in order to keep the dispatch code in typedUnnamed as small as possible. That often helps the JIT to opimize.

@odersky odersky assigned smarter and unassigned odersky Apr 21, 2020
@smarter smarter force-pushed the disallow-tparam-questionmark branch from d08b906 to 4a71417 Compare April 23, 2020 20:10
Now that we use `?` for wildcards, we should treat it more like a
keyword for types and not allow it as a type name to avoid
confusion.
@smarter smarter force-pushed the disallow-tparam-questionmark branch from 4a71417 to 30aa58d Compare April 23, 2020 20:11
@smarter smarter merged commit ed35255 into scala:master Apr 23, 2020
@smarter smarter deleted the disallow-tparam-questionmark branch April 23, 2020 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants